Ensure CI from external MI and foreign abs interpreter survive precompilation#60747
Ensure CI from external MI and foreign abs interpreter survive precompilation#60747
Conversation
Co-authored-by: Valentin Churavy <vchuravy@mit.edu> (cherry picked from commit b33ce72)
vchuravy
force-pushed
the
vc+gb/external_precompilation
branch
from
17711b9 to
9a8e5d2
Compare
|
I expect that we also need: diff --git a/src/staticdata_utils.c b/src/staticdata_utils.c
index 16ebcd4b4ee..ad6aceb411e 100644
--- a/src/staticdata_utils.c
+++ b/src/staticdata_utils.c
@@ -502,7 +502,7 @@ static jl_array_t *queue_external_cis(jl_array_t *list, jl_query_cache *query_ca
int dispatch_status = jl_atomic_load_relaxed(&m->dispatch_status);
if (!(dispatch_status & METHOD_SIG_LATEST_WHICH))
continue; // ignore replaced methods
- if (ci->owner == jl_nothing && jl_atomic_load_relaxed(&ci->inferred) && jl_is_method(m) && jl_object_in_image((jl_value_t*)m->module)) {
+ if (jl_atomic_load_relaxed(&ci->inferred) && jl_is_method(m) && jl_object_in_image((jl_value_t*)m->module)) {
int found = has_backedge_to_worklist(mi, &visited, &stack, query_cache);
assert(found == 0 || found == 1 || found == 2);
assert(stack.len == 0);but I want to ensure that the new test fails. |
|
|
||
| include("precompile_utils.jl") | ||
|
|
||
| precompile_test_harness() do load_path |
There was a problem hiding this comment.
@aviatesk I added a new test here instead of adding to precopile_absintX since the structure is slightly different.
The test comes from
https://github.com/JuliaGPU/GPUCompiler.jl/blob/master/test/native/precompile.jl
where we have a "compiler" package and a "user" package, and we are adding a code instance to a method instance that is already owned somewhere else identity(::Any) which is why we need the PrecompileTools-esque setup.
test/precompile_utils.jl
Outdated
| function check_presence(mi, token) | ||
| ci = isdefined(mi, :cache) ? mi.cache : nothing | ||
| while ci !== nothing | ||
| if ci.owner === token && ci.max_world == typemax(UInt) |
There was a problem hiding this comment.
Something probably went horribly wrong if ci.max_world != WORLD_AGE_REVALIDATION_SENTINEL, since that means the Compiler made a mistake and forgot to get it validated
Co-authored-by: Jameson Nash <vtjnash@gmail.com>
Co-authored-by: Jameson Nash <vtjnash@gmail.com>
| ci = isdefined(mi, :cache) ? mi.cache : nothing | ||
| while ci !== nothing | ||
| if ci.owner === token | ||
| @test ci.max_world == Base.ReinferUtils.WORLD_AGE_REVALIDATION_SENTINEL || ci.min_world <= 1 |
There was a problem hiding this comment.
@vtjnash I am a bit confused about this query:
- There could be more than one entry? We should select the one that is valid in the current world.
The values I am observing here are:
ci = CodeInstance for MethodInstance for ExampleUser.square(::Float64)
ci.max_world = 0xffffffffffffffff
ci.min_world = 0x0000000000009b8c
ci = CodeInstance for MethodInstance for identity(::Any)
ci.max_world = 0xffffffffffffffff
ci.min_world = 0x0000000000009b8c
Which does look to me like they are correctly integrated? But they of course fail the test, since max_world != WORLD_AGE_REVALIDATION_SENTINAL and min_world is not <=1
|
@MasonProtter given #60442 could you write some tests that check that foreign code instances that are part of a |
4 participants
Forward-port of #60741